-
-
Notifications
You must be signed in to change notification settings - Fork 200
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added an html validation check by looking at Nokogiri errors #125
Conversation
All right, I am less grumpy about this now. Plus I like that it's a separate, non-default option. Can you update the README with the new option? Everything else looks grand. |
svg audio embed source track video) | ||
|
||
def run | ||
return unless @options[:validate_html] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this superfluous?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In what sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, because get_checks
would've deleted it above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. If I want to run this check directly, I shouldn't need to know about the validate_html
option, should I? If I'm calling it directly, the understanding would be that validate_html
would be true
. This is useful if I want to validate the HTML in some documents, but not all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, you are correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I copied this from the favicon code:
return unless @options[:favicon] |
Do you still want me to remove it here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think both should be removed
3b832ec
to
f21ff1f
Compare
Added docs to readme, let me know if there's something else I can do. |
f21ff1f
to
e413f18
Compare
e413f18
to
7d7068e
Compare
@akoeplinger I would agree that the favicon |
@gjtorikian sorry if that sounds pedantic, but I'd like to keep this PR focused on the new feature and not change unrelated code. |
😌 👍 🎉 Glad this is making its way in! |
Added an html validation check by looking at Nokogiri errors
I know you said in #30 you're not interested in validating HTML, but Nokogiri already looks at the markup and provides errors when parsing, so providing basic validation is quite easy.
This simple checker helped me catch a few mistakes in my site, so I thought I'd try and see if you find it useful now 😄